sEPD Event Plane Calibration - Application#4127
Merged
pinkenburg merged 11 commits intosPHENIX-Collaboration:masterfrom Feb 24, 2026
Merged
sEPD Event Plane Calibration - Application#4127pinkenburg merged 11 commits intosPHENIX-Collaboration:masterfrom
pinkenburg merged 11 commits intosPHENIX-Collaboration:masterfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new EventPlaneRecov2 SubsysReco and Eventplaneinfov2 container for event-plane calibration/Q‑vector handling, extends Eventplaneinfo interface, includes SEPD in CaloTowerStatus hot-map selection, and updates build files to expose new headers/sources and ROOT dictionaries. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Recov as EventPlaneRecov2
participant CDB as CDB/Calib
participant EH as EventHeader
participant SEPD as SEPD_Towers
participant Cent as CentralityInfo
participant DST as DST/Nodes
Client->>Recov: Init(topNode)
Recov->>CDB: Load calibration (direct URL or calib dir)
CDB-->>Recov: CorrectionData per harmonic/cent_bin
Recov->>Recov: LoadCalib() (store averages & X-matrices)
Client->>Recov: process_event(topNode)
Recov->>EH: Read global event index
Recov->>Cent: Read CentralityInfo
Cent-->>Recov: cent_bin
Recov->>SEPD: process_sEPD() (read towers, accumulate charges)
SEPD-->>Recov: raw Q-vectors (S, N, NS)
Recov->>Recov: correct_QVecs() (recenter then apply flattening matrices)
Recov->>DST: FillNode(topNode) (create/insert EventplaneinfoMap entries)
DST-->>Recov: confirm insertion
Client->>Recov: ResetEvent(topNode)
Recov->>Recov: clear per-event state
Possibly related PRs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Build & test reportReport for commit eb122648a02f13af194f74a9af44e64934e2ea02:
Automatically generated by sPHENIX Jenkins continuous integration |
Contributor
|
There will be more comments but why do you add a version 2 of the reconstruction class? As far as I know this here is a replacement - is there any point in running the old version? |
Contributor
Author
Hi Chris, yes this will be a replacement for the
I did not want to just outright delete and replace the contents of |
Build & test reportReport for commit 760d32daa74846e600de6a512c93824da8d4f684:
Build & test reportReport for commit 561e967652518788cb59cd7b23a726994d5bef0e:
Build & test reportReport for commit 3fa48da17176b340a61716e6a0ab2aa69ea4894d:
Build & test reportReport for commit 9ec298fb7d3b94a243e8056445b7879d11f58659:
Build & test reportReport for commit 18c5a2c56900618bcf9798bb091f064b628994a4:
Build & test reportReport for commit f704495167ff5a179e3b400f49850351b7084714:
Automatically generated by sPHENIX Jenkins continuous integration Automatically generated by sPHENIX Jenkins continuous integration Automatically generated by sPHENIX Jenkins continuous integration Automatically generated by sPHENIX Jenkins continuous integration Automatically generated by sPHENIX Jenkins continuous integration Automatically generated by sPHENIX Jenkins continuous integration |
Build & test reportReport for commit 1e206178d882a77d662fe8517b6f9901b826eab1:
Automatically generated by sPHENIX Jenkins continuous integration |
There was a problem hiding this comment.
Pull request overview
This PR adds an sEPD-specific event-plane reconstruction and calibration application, wiring it to the new CDB-based Q-vector calibration payloads and extending the event-plane info container to hold multiple calibration stages.
Changes:
- Introduces
Eventplaneinfov2, extending the event-plane info interface to hold raw, recentered, and final (flattened) Q-vectors and derived angles. - Adds
EventPlaneRecov2, aSubsysRecomodule that computes sEPD Q-vectors for harmonics 2–4, applies centrality-binned recentering and 2×2 flattening matrices from CDBTTree, and fills theEventplaneinfoMapwithEventplaneinfov2nodes. - Extends
CaloTowerStatushandling so SEPD uses the_BadTowerMapconvention for hot/bad channel maps, aligning sEPD operations with CEMC.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
offline/packages/eventplaneinfo/Makefile.am |
Registers Eventplaneinfov2 and EventPlaneRecov2 headers, implementations, and ROOT dictionaries with the build, so the new container and module are compiled and available. |
offline/packages/eventplaneinfo/Eventplaneinfov2LinkDef.h |
Adds ROOT CINT dictionary directives for Eventplaneinfov2, enabling persistence and I/O via ROOT. |
offline/packages/eventplaneinfo/Eventplaneinfov2.h |
Defines Eventplaneinfov2 as a subclass of Eventplaneinfo, adding storage and accessors for raw, recentered, and flattened Q-vectors, plus ring-resolved Q-vectors and psi accessors. |
offline/packages/eventplaneinfo/Eventplaneinfov2.cc |
Implements identify and a concrete GetPsi method (handling the Q=(0,0) case with NaN), providing core angle computation for the new container. |
offline/packages/eventplaneinfo/Eventplaneinfo.h |
Extends the base interface to include virtual hooks for raw and recentered Q-vectors, so downstream code can use these in a type-agnostic way via Eventplaneinfo*. |
offline/packages/eventplaneinfo/EventPlaneRecov2.h |
Declares the new EventPlaneRecov2 SubsysReco module, including configuration knobs, CDBTTree handle, correction-data structures, and internal Q-vector state for harmonics 2–4 across S/N/NS. |
offline/packages/eventplaneinfo/EventPlaneRecov2.cc |
Implements the sEPD event-plane reconstruction pipeline: loads per-harmonic, per-centrality recentering and flattening data from CDB, computes charge-weighted Q-vectors from TOWERINFO_CALIB_SEPD, applies recentering and 2×2 matrix flattening, and populates EventplaneinfoMap with Eventplaneinfov2 records for sEPDS, sEPDN, and sEPDNS (including raw and recentered vectors). |
offline/packages/eventplaneinfo/Eventplaneinfo.cc (unchanged content, but used by new code) |
Existing implementation remains the common base; new v2 class plugs into this hierarchy without altering behavior of v1 users. |
offline/packages/CaloReco/CaloTowerStatus.cc |
Adjusts hot-map calibration name selection so SEPD uses the same _BadTowerMap naming convention as CEMC, allowing SEPD bad tower maps produced by the calibration pipeline to be picked up automatically. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Build & test reportReport for commit 67ab2c156e2c3998324091661011e038f35cb04c:
Automatically generated by sPHENIX Jenkins continuous integration |
Introduces a new sEPD event plane reconstruction module (v2) and an expanded info container (v2) to support a full calibration chain including recentering and flattening.
Eventplaneinfov2:
- Inherits from Eventplaneinfo.
- Adds dedicated storage for raw, recentered, and final (flattened) Q-vectors.
- Provides getters/setters for extended Q-vector stages to allow for systematic studies of calibration effects.
EventPlaneRecov2:
- New SubsysReco module specifically for sEPD Q-vector calibration.
- Implements a multi-step calibration workflow:
1. Raw Q-vector calculation from sEPD channel charges and geometry.
2. Recentering: Subtracts <Q> offsets per centrality bin.
3. Flattening: Applies a 2x2 correction matrix calculated from Q-vector variances (Qxx, Qyy, Qxy).
- Features integration with CDBTTree to load calibration parameters dynamically via the Calibration Database.
- Processes harmonics n=2,3, and 4 for South, North, and Combined (NS) subdetectors.
- Automatically handles centrality-based binning (8 bins) for calibrations.
- Populates EventplaneinfoMap with Eventplaneinfov2 objects for downstream analysis.
- Includes diagnostic verbosity levels for inspecting calibration matrices and per-event vector transformations.
- Allow the sEPD bad tower maps to be processed via the CaloTowerStatus module.
Integrate the "Combined NS" calibration strategy into the reconstruction pipeline to improve event plane resolution and flatness. This update treats the combined North-South detector as a distinct third entity for flattening, while preserving multiplicity-based resolution weighting through individual sub-detector recentering. Core Logic Updates: - Calibration Loading: Updated LoadCalib to retrieve North-South specific second moments (⟨Qx2⟩, ⟨Qy2⟩, ⟨QxQy⟩) from the Calibration Database (CDB). - Matrix Computation: Implemented unique flattening matrix calculation for the NS detector slot (Index 2) during initialization. 1) "Best of Both Worlds" Correction: Refactored correct_QVecs to implement the optimized combination logic: 2) Recenters South and North vectors individually. 3) Sums the recentered components to form the combined recentered vector: Q_NS,rec = Q_S,rec + Q_N,rec. - Applies the dedicated NS flattening matrix to the resulting sum to ensure a circular (flat) distribution. - Raw Vector Summation: Updated process_sEPD to populate the raw NS vector by summing normalized sub-detector components, facilitating direct QA comparisons. Infrastructure & Efficiency: - Array Expansion: Increased internal storage arrays for m_correction_data, m_Q_raw, m_Q_recentered, and m_Q_flat from size 2 to size 3 to accommodate the NS detector index. Diagnostics & Quality Assurance: - Print Methods: Expanded print_correction_data and print_QVectors to display diagnostic information for all three detector slots (South, North, and NorthSouth). - Fallback Safety: Maintained fallback logic that utilizes raw Q-vectors in the event of missing calibration data or invalid event centrality, preventing empty output nodes.
Address Major Issues: - Ensure m_directURL_EventPlaneCalib is not empty before passing it to hasValidTree - Removed unused vectors in EventPlaneRecov2 (south_psi, north_psi, northsouth_psi) - Guard against order == 0 to avoid divide-by-zero. - Add bounds checks before indexing Q-vector storage.
Steepspace
force-pushed
the
sEPD-Calib
branch
from
67ab2c1 to
8b5c354
Compare
Build & test reportReport for commit 8b5c354c39a7ed64cae634b3bc9139e6a699c85d:
Automatically generated by sPHENIX Jenkins continuous integration |
…se problems and is counter productive here, remove not needed static casts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
offline/packages/eventplaneinfo/EventPlaneRecov2.cc (1)
38-38:⚠️ Potential issue | 🟠 Major
#include <format>requires C++20 — build may fail.This file includes
<format>and usesstd::formatextensively (~37 call sites) as well asstd::ranges::max_element(line 513). The project'sconfigure.acdoes not enforce-std=c++20. If the default compiler standard is C++17, this will not compile.Consider replacing
std::formatwithstd::ostringstreamor a lightweight formatting approach (e.g.,boost::formator thefmtlibrary, which is the reference implementation forstd::formatand works under C++11/14/17).
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
offline/packages/CaloReco/CaloTowerStatus.ccoffline/packages/eventplaneinfo/EventPlaneRecov2.ccoffline/packages/eventplaneinfo/EventPlaneRecov2.hoffline/packages/eventplaneinfo/Eventplaneinfo.hoffline/packages/eventplaneinfo/Eventplaneinfov2.ccoffline/packages/eventplaneinfo/Eventplaneinfov2.hoffline/packages/eventplaneinfo/Eventplaneinfov2LinkDef.hoffline/packages/eventplaneinfo/Makefile.am
🚧 Files skipped from review as they are similar to previous changes (1)
- offline/packages/CaloReco/CaloTowerStatus.cc
Comment on lines
+48
to
+86
| int EventPlaneRecov2::Init(PHCompositeNode *topNode) | ||
| { | ||
| std::string calibdir = CDBInterface::instance()->getUrl(m_calibName); | ||
|
|
||
| if (!m_directURL_EventPlaneCalib.empty()) | ||
| { | ||
| m_cdbttree = new CDBTTree(m_directURL_EventPlaneCalib); | ||
| std::cout << PHWHERE << " Custom Event Plane Calib Found: " << m_directURL_EventPlaneCalib << std::endl; | ||
| } | ||
| else if (!calibdir.empty()) | ||
| { | ||
| m_cdbttree = new CDBTTree(calibdir); | ||
| std::cout << PHWHERE << " Event Plane Calib Found: " << calibdir << std::endl; | ||
| } | ||
| else if (m_doAbortNoEventPlaneCalib) | ||
| { | ||
| std::cout << PHWHERE << " Error: No Event Plane Calib Found and m_doAbortNoEventPlaneCalib is true. Aborting." << std::endl; | ||
| return Fun4AllReturnCodes::ABORTRUN; | ||
| } | ||
| else | ||
| { | ||
| std::cout << PHWHERE << " Error: No Event Plane Calib Found. Skipping Event Plane Calibrations." << std::endl; | ||
| m_doNotCalib = true; | ||
| } | ||
|
|
||
| if (!m_doNotCalib) | ||
| { | ||
| LoadCalib(); | ||
| } | ||
|
|
||
| if (Verbosity() > 0) | ||
| { | ||
| print_correction_data(); | ||
| } | ||
|
|
||
| CreateNodes(topNode); | ||
|
|
||
| return Fun4AllReturnCodes::EVENT_OK; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the EventPlaneRecov2 files to understand the complete context
find . -name "EventPlaneRecov2*" -type f | head -20Repository: sPHENIX-Collaboration/coresoftware
Length of output: 184
🏁 Script executed:
# Check if EventPlaneRecov2 has an InitRun() implementation
grep -n "InitRun" $(find . -name "EventPlaneRecov2*" -type f) 2>/dev/null | head -20Repository: sPHENIX-Collaboration/coresoftware
Length of output: 60
🏁 Script executed:
# Find CreateNodes implementation to see what it returns
grep -n "CreateNodes" $(find . -name "EventPlaneRecov2*" -type f) 2>/dev/nullRepository: sPHENIX-Collaboration/coresoftware
Length of output: 385
🏁 Script executed:
# Look at standard pattern: check how other SubsysReco/reconstruction modules handle CDB
echo "=== Searching for CDB loading patterns in reconstruction modules ==="
rg "CDBInterface.*getUrl|CDBTTree" --type cpp -B 5 -A 5 | grep -E "(Init\(|InitRun\(|getUrl|CDBTTree)" | head -50Repository: sPHENIX-Collaboration/coresoftware
Length of output: 5161
🏁 Script executed:
# Find all files that use both CDBInterface and have InitRun defined
echo "=== Modules with InitRun and CDB usage ==="
find . -name "*.cc" -type f | xargs grep -l "InitRun" | head -10 | while read f; do
if grep -q "CDBInterface\|CDBTTree\|getUrl" "$f"; then
echo "File: $f"
grep -n "int.*InitRun\|int.*Init\|getUrl\|CDBTTree" "$f" | head -10
echo "---"
fi
doneRepository: sPHENIX-Collaboration/coresoftware
Length of output: 1316
🏁 Script executed:
# Check if EventPlaneRecov2 declares/implements InitRun
cat -n ./offline/packages/eventplaneinfo/EventPlaneRecov2.h | head -100Repository: sPHENIX-Collaboration/coresoftware
Length of output: 3301
🏁 Script executed:
# Check what function in EventPlaneRecov2.cc loads CDB and in which function
grep -n "int EventPlaneRecov2::" ./offline/packages/eventplaneinfo/EventPlaneRecov2.cc | head -20Repository: sPHENIX-Collaboration/coresoftware
Length of output: 529
🏁 Script executed:
# More specifically, check the Init and InitRun signatures in the header
grep -A 2 -B 2 "Init" ./offline/packages/eventplaneinfo/EventPlaneRecov2.hRepository: sPHENIX-Collaboration/coresoftware
Length of output: 209
🏁 Script executed:
# Now check ZdcReco to see if it's a counter-example (CDB in Init not InitRun)
grep -n "int.*Init\|getUrl\|CDBTTree" ./offline/packages/zdcinfo/ZdcReco.cc | head -20Repository: sPHENIX-Collaboration/coresoftware
Length of output: 297
🏁 Script executed:
# Check the structure of one of the standard pattern modules more carefully
echo "=== emcNoisyTowerFinder pattern ==="
grep -n "int.*Init" ./calibrations/calorimeter/calo_emc_noisy_tower/emcNoisyTowerFinder.ccRepository: sPHENIX-Collaboration/coresoftware
Length of output: 180
🏁 Script executed:
# Check the CreateNodes function return value handling
sed -n '249,275p' ./offline/packages/eventplaneinfo/EventPlaneRecov2.ccRepository: sPHENIX-Collaboration/coresoftware
Length of output: 974
🏁 Script executed:
# Check what happens at line 83 in Init where CreateNodes is called
sed -n '48,90p' ./offline/packages/eventplaneinfo/EventPlaneRecov2.ccRepository: sPHENIX-Collaboration/coresoftware
Length of output: 1351
🏁 Script executed:
# Check the event processing to see if it would crash if EventplaneinfoMap doesn't exist
sed -n '504,520p' ./offline/packages/eventplaneinfo/EventPlaneRecov2.ccRepository: sPHENIX-Collaboration/coresoftware
Length of output: 1244
Ignoring CreateNodes() return value will mask initialization errors
Line 83 calls CreateNodes(topNode) but discards its return value. CreateNodes() returns ABORTRUN when the DST node is missing, but Init() proceeds to return EVENT_OK anyway. This silently masks critical errors—if DST doesn't exist, the module will initialize as if successful, only to crash later in FillNode() when trying to access the EventplaneinfoMap. Check and propagate the return value:
- CreateNodes(topNode);
-
- return Fun4AllReturnCodes::EVENT_OK;
+ return CreateNodes(topNode);CDB calibrations should be loaded in InitRun(), not Init()
The standard pattern across this codebase (ZdcReco, emcNoisyTowerFinder, InttCalib, etc.) is to load CDB calibrations in InitRun(), where the run number is guaranteed to be available. Calliing CDBInterface::instance()->getUrl() in Init() before the run is known can return stale or incorrect calibration URLs. Implement InitRun() and move the calibration-loading logic there.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| int EventPlaneRecov2::Init(PHCompositeNode *topNode) | |
| { | |
| std::string calibdir = CDBInterface::instance()->getUrl(m_calibName); | |
| if (!m_directURL_EventPlaneCalib.empty()) | |
| { | |
| m_cdbttree = new CDBTTree(m_directURL_EventPlaneCalib); | |
| std::cout << PHWHERE << " Custom Event Plane Calib Found: " << m_directURL_EventPlaneCalib << std::endl; | |
| } | |
| else if (!calibdir.empty()) | |
| { | |
| m_cdbttree = new CDBTTree(calibdir); | |
| std::cout << PHWHERE << " Event Plane Calib Found: " << calibdir << std::endl; | |
| } | |
| else if (m_doAbortNoEventPlaneCalib) | |
| { | |
| std::cout << PHWHERE << " Error: No Event Plane Calib Found and m_doAbortNoEventPlaneCalib is true. Aborting." << std::endl; | |
| return Fun4AllReturnCodes::ABORTRUN; | |
| } | |
| else | |
| { | |
| std::cout << PHWHERE << " Error: No Event Plane Calib Found. Skipping Event Plane Calibrations." << std::endl; | |
| m_doNotCalib = true; | |
| } | |
| if (!m_doNotCalib) | |
| { | |
| LoadCalib(); | |
| } | |
| if (Verbosity() > 0) | |
| { | |
| print_correction_data(); | |
| } | |
| CreateNodes(topNode); | |
| return Fun4AllReturnCodes::EVENT_OK; | |
| } | |
| int EventPlaneRecov2::Init(PHCompositeNode *topNode) | |
| { | |
| std::string calibdir = CDBInterface::instance()->getUrl(m_calibName); | |
| if (!m_directURL_EventPlaneCalib.empty()) | |
| { | |
| m_cdbttree = new CDBTTree(m_directURL_EventPlaneCalib); | |
| std::cout << PHWHERE << " Custom Event Plane Calib Found: " << m_directURL_EventPlaneCalib << std::endl; | |
| } | |
| else if (!calibdir.empty()) | |
| { | |
| m_cdbttree = new CDBTTree(calibdir); | |
| std::cout << PHWHERE << " Event Plane Calib Found: " << calibdir << std::endl; | |
| } | |
| else if (m_doAbortNoEventPlaneCalib) | |
| { | |
| std::cout << PHWHERE << " Error: No Event Plane Calib Found and m_doAbortNoEventPlaneCalib is true. Aborting." << std::endl; | |
| return Fun4AllReturnCodes::ABORTRUN; | |
| } | |
| else | |
| { | |
| std::cout << PHWHERE << " Error: No Event Plane Calib Found. Skipping Event Plane Calibrations." << std::endl; | |
| m_doNotCalib = true; | |
| } | |
| if (!m_doNotCalib) | |
| { | |
| LoadCalib(); | |
| } | |
| if (Verbosity() > 0) | |
| { | |
| print_correction_data(); | |
| } | |
| return CreateNodes(topNode); | |
| } |
Build & test reportReport for commit 37a596e5a70cd94be99288a7a3db2817844c85f8:
Automatically generated by sPHENIX Jenkins continuous integration |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Types of changes
What kind of change does this PR introduce? (Bug fix, feature, ...)
Introduces a new sEPD event plane reconstruction module (v2) and an expanded info container (v2) to support a full calibration chain including recentering and flattening.
Eventplaneinfov2:
EventPlaneRecov2:
Presentations:
TODOs (if applicable)
Links to other PRs in macros and calibration repositories (if applicable)
sEPD Q Vector Calibration Generation: #4106
sEPD Q Vector Calibration Generation (macros): sPHENIX-Collaboration/macros#1270
sEPD Event Plane Calibration – Application (PR #4127)
Motivation & Context
Enable a full, centrality‑dependent sEPD Q‑vector calibration and event‑plane production chain to improve event‑plane accuracy and support systematic studies. Moves calibration storage from many TProfile histograms to a compact CDBTTree format, adds recentering and 2×2 flattening corrections applied directly to Q‑vectors, and provides a v2 reconstruction implementation while preserving the legacy class for backward compatibility.
Key changes
New eventplane data type: Eventplaneinfov2
New reconstruction module: EventPlaneRecov2 (SubsysReco)
Build / integration
Implementation notes
Potential risk areas
Possible future improvements
Note: AI-generated summaries can contain mistakes. Reviewers should inspect Q‑vector math, calibration loading behavior, node population, and run validation with real calibration data before adoption.